Skip to content

fix(observe): replace undefined-sentinel with boolean flags in withBatch and withDeduplicateData#127

Merged
krisnye merged 2 commits into
mainfrom
krisnye/undefined-sentinel
Jun 24, 2026
Merged

fix(observe): replace undefined-sentinel with boolean flags in withBatch and withDeduplicateData#127
krisnye merged 2 commits into
mainfrom
krisnye/undefined-sentinel

Conversation

@krisnye

@krisnye krisnye commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Two observe operators used undefined as a "not yet emitted" sentinel, which caused silent data loss when undefined was a legitimate value.

withBatch — the microtask flush guard if (pendingValue !== undefined) silently dropped any batched emission of undefined. Fixed by replacing with a hasPendingValue: boolean flag.

withDeduplicateData — the first-emission check lastValue === undefined would fail to deduplicate repeated undefined emissions if undefined bypassed the T extends Data constraint (e.g. via any or generics). Fixed by replacing with a notified: boolean flag, matching the pattern already used in withDeduplicate, withCache, and withOptional.

Both fixes follow the existing convention in the package: track emission state with a boolean, never overload the data slot.

Related PRs

None.

krisnye and others added 2 commits June 23, 2026 10:15
…tch and withDeduplicateData

withBatch dropped batched `undefined` values via `pendingValue !== undefined`
guard; withDeduplicateData failed to deduplicate repeated `undefined` via
`lastValue === undefined` sentinel. Both now use dedicated boolean flags.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…undefined

U must not include undefined because undefined is reserved as the skip sentinel.
Type test verifies that explicit U = T | undefined is rejected at compile time.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@krisnye krisnye merged commit 90c42d2 into main Jun 24, 2026
3 checks passed
@krisnye krisnye deleted the krisnye/undefined-sentinel branch June 24, 2026 16:21
krisnye added a commit that referenced this pull request Jun 24, 2026
…ype param

withFilter gained U extends {} | null in #127; useDragTransaction's unconstrained
T no longer satisfies it. Constraining T here aligns the type throughout the
call chain and reflects the genuine semantic — a transaction arg of undefined
makes no sense.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
krisnye added a commit that referenced this pull request Jun 24, 2026
* fix(ci): extend gate to cover all published downstream packages

The CI gate only checked @adobe/data; data-lit's type error introduced
in #127 was invisible to CI and reached main undetected.

Two jobs now:
- data: existing @adobe/data typecheck + lint + test (unchanged)
- packages: depends on data; rebuilds @adobe/data for dist/, then
  builds (tsc -b) and tests all @adobe/data-* packages. data-persistence
  uses test:node to avoid the Playwright browser requirement in CI.

Also fixes the latent data-lit error: useDragTransaction<T> now
constrains T extends {} | null to satisfy withFilter's U bound
(tightened in #127).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(data-persistence): open node-fs files with O_RDWR|O_CREAT, not a+

"a+" (append+read) ignores the position argument on writes — every
handle.write() lands at EOF regardless of the offset passed. This made
writeAt semantically equivalent to appendAt, so all conformance tests
that write at a non-zero offset failed.

O_RDWR|O_CREAT (read+write, create-if-missing, never truncate) is the
correct flag set: positions are honoured on write, the file is created
on first open, and existing content is never clobbered.

These tests were never run in CI before the packages job was added in
this branch, so the bug went undetected since #102.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant